add new methods to the NodePrinter to enable range tracking possibilities when demangling a name #82336
Conversation
charles-zablit
added
the
demangling
|
Please note that this PR should be merged after: Only the changes in e9219cf541a98f17bd500d07b547f0fc1504f145 are relevant to this PR. |
charles-zablit
force-pushed
the
charles-zablit/add-new-demangling-methods
branch
from
a98b2e6 to
e9219cf
Compare
|
@swift-ci please test |
|
@swift-ci please test windows |
|
@swift-ci please test macOS |
|
@swift-ci please test Windows |
…ties when demangling a name
charles-zablit
force-pushed
the
charles-zablit/add-new-demangling-methods
branch
from
e9219cf to
479e664
Compare
|
@swift-ci please test |
include/swift/Demangling/Demangle.h
Outdated
| /// | ||
| /// \returns The demangled string. | ||
| void demangleSymbolAsString(llvm::StringRef MangledName, | ||
| NodePrinter *printer); |
There was a problem hiding this comment.
Why does this take a NodePrinter * instead of a NodePrinter &? Is there any meaningful use for passing in a nullptr here?
There was a problem hiding this comment.
Why does this variant not take DemangleOptions? Are they owned by NodePrinter?
There was a problem hiding this comment.
Why does this take a
NodePrinter *instead of aNodePrinter &? Is there any meaningful use for passing in anullptrhere?
Currently there is no reason for it to take a pointer instead of a reference. I switched to using references. Initially, the API accepted optional NodePrinter, but that is no longer the case.
Why does this variant not take
DemangleOptions? Are they owned byNodePrinter?
DemangleOptions is used to init the NodePrinter later on. In this variant, we pass an already initialized NodePrinter.
include/swift/Demangling/Demangle.h
Outdated
| /// \param mangledNameLength The length of the mangledName string. | ||
| /// \param printer The NodePrinter that will be used to demangle the symbol. | ||
| void demangleSymbolAsString(const char *mangledName, size_t mangledNameLength, | ||
| NodePrinter *printer); |
There was a problem hiding this comment.
Since this is a new API anyway... Do we access to llvm::StringRef here? Or is this meant to be compatible to a C API?
There was a problem hiding this comment.
If the latter, then how would someone create a NodePrinter object? And if it's not needed, maybe we can get rid of this variant altogether?
There was a problem hiding this comment.
I followed the previous implementation of demangleSymbolAsString, but these 2 variants are not used. I removed them. The remaining variants are the llvm::StringRef ones.
If the latter, then how would someone create a NodePrinter object?
We create and use it in swiftlang/llvm-project#10710:
TrackingNodePrinter printer = TrackingNodePrinter(options);
swift::Demangle::demangleSymbolAsString(symbol, printer);
return std::pair<std::string, std::optional<DemangledNameInfo>>(
printer.takeString(), printer.takeInfo());
|
@swift-ci please test |
2 participants
This PR implements range tracking for the name and parameters of a demangled function.
Motivation
In this patch, @Michael137 implemented name highlighting for methods in the C++ plugin of LLDB. This results in better readability when reading backtraces of functions with long scopes.
I am working on implementing the same feature for the Swift plugin in LLDB. Please see an example below:
Before:
After:
Notice how the name and parameters of the functions are highlighted differently than the rest.
(Please note that the difference in text contents are temporary, and should disappear before for the final patch).
Implementation details
To implement this feature in the Swift plugin of LLDB, we need to provide LLDB with the ranges of the "components" of the demangled name. For instance the
baseNameofbar.foo()spans4...7, while theparametersspan7...9.This information allows LLDB to build the name that is displayed in the backtraces.
Original implementation
This patch introduces a new class called
TrackingDemanglerPrinterwhich stores the information needed to track the ranges. It can later be extended to track more ranges. The methods it defines are called in theprintmethod ofNodePrinter.cpp.The previous behavior is still available when calling
DemangleSymbolAsStringwithout aprinter, as this will result in a no op whenstartNameis called for instance.Current implementation
2 methods were made virtual in
NodePrinter, so that any consumer can override them and track the ranges they need. This is less lldb specific than the previous method.Testing (removed, relevant only for the original implementation)
To implement the tests for the range tracking, I used the
manglings.txtfile. It now contains the ranges for the name and parameters where that's relevant. The format I chose to serialize the ranges is arbitrary and I'm happy to discuss any change, especially when considering we might want to add more tracked ranges.I also developed a Python script to help review the test cases by coloring them. Please find it here for reference. It could be interesting to add it to the repo.
Follow ups
swift-demangle, as these changes make it fairly trivial to implement, and it would greatly help with readability.Please note that it was originally opened here #81511 but I moved to a different target branch.